Fix chain methods for XComArg#10827
Conversation
|
@casassg would like to take a look? |
kaxil
left a comment
kaxil
left a comment
There was a problem hiding this comment.
Good spot @yuqian90 , thanks for the fix @turbaszek
yuqian90
left a comment
yuqian90
left a comment
There was a problem hiding this comment.
Thanks for the quick fix. One other issue was that
There was a problem hiding this comment.
Thanks for the quick fix. One other issue is that if we do this at this line:
xcom_args_a >> bash_op1 >> xcom_args_b
Is this supposed to work? i have not tried yet, but looking at the code, I think BaseOperator.__rshift__ will complain that it can't handle xcom_args. That's a different issue though so the fix can be done separately.
There was a problem hiding this comment.
Same DAG is used in tests to make sure that it works. BaseOperator._set_relatives handles XComArgs properly. However, XComArgs is missing in type hints due to cyclic imports. I think that once we have your PR merged we may consider something like TaskType that will be union of types that are "task-like"
casassg
left a comment
casassg
left a comment
There was a problem hiding this comment.
This is great, btw, should we also add support for __rrshift__ ? https://github.com/apache/airflow/blob/master/airflow/models/baseoperator.py#L510
__lshift__ and __rshift__ methods should return other not self. This PR fixes XComArg implementation to support chain like this one: BaseOprator >> XComArg >> BaseOperator Related to: apache#10153
turbaszek
force-pushed
the
fix-chain-for-xcom-args
branch
from
a014f5b to
6462c29
Compare
|
@casassg @yuqian90 I've added missing method. I'm wondering about making |
Sounds like a prime use of a mixin to me, rather than a base class. But some DRYing here now we have it three places definitely sounds good. |
5 participants
lshift and rshift methods should return other not self.
This PR fixes XComArg implementation to support chain like this one:
BaseOprator >> XComArg >> BaseOperator
Related to: #10153
Kudos to @yuqian90 for spotting this issue! 🚀
I --- **^ Add meaningful description above**Read the Pull Request Guidelines for more information.
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.